Skip to content

Feature/edr server#11

Merged
jdw-creare merged 10 commits intodevelopfrom
feature/edr_server
Dec 12, 2025
Merged

Feature/edr server#11
jdw-creare merged 10 commits intodevelopfrom
feature/edr_server

Conversation

@scranford1
Copy link
Copy Markdown

  • Initial implementation of the Environmental Data Retrieval (EDR) standard for the OGC server.

@scranford1 scranford1 requested a review from jdw-creare December 8, 2025 15:52
Copy link
Copy Markdown
Contributor

@jdw-creare jdw-creare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all looks great! I had a couple comments, but I feel like it already meets the bar for merging. Feel free to merge at any time!

Comment on lines +26 to +44
@property
def api(self) -> pygeoapi.api.API:
"""Property for the API created using a custom configuration.

Returns
-------
pygeoapi.api.API
The API which handles all EDR requests.
"""
# Allow specifying GeoTiff or CoverageJSON in the format argument.
# This is a bypass which is needed to get by a conditional check in pygeoapi.
pygeoapi.plugin.PLUGINS["formatter"]["GeoTiff"] = ""
pygeoapi.plugin.PLUGINS["formatter"]["CoverageJSON"] = ""

config = EdrConfig.get_configuration(self.base_url, self.layers)
open_api = get_oas(config, fail_on_invalid_collection=False)
EdrProvider.set_resources(self.layers)
return pygeoapi.api.API(config=deepcopy(config), openapi=open_api)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the pygeoapi.api.API object something we can safely create once on startup and re-use every request? I'm not sure how long it takes to create, but I click into that constructor and see methods like "load_plugin" and "deep_copy", which sometimes are the kind of thing that has a non-trivial load time. If we can safely re-use it, it might be a free performance gain.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I think my initial thought was that if the layers change the API needs to be updated to match it. But it may be worth trying to find a better way to make that happen.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to observe layer changes and only update the API when a layer change occurs.

Comment on lines +23 to +25
base_url = tl.Unicode(default_value="http://127.0.0.1:5000/")
layers = tl.List(trait=tl.Instance(pogc.Layer), default_value=[])

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you wind up instantiating the pygeoapi.api.API object just once, then these could become just parameters to __init__, just to keep this class as stateless as possible

@sonarqubecloud
Copy link
Copy Markdown

@scranford1
Copy link
Copy Markdown
Author

Updates Based on Review:

  • Fix strict slashes.
  • Change CRS interpreter to throw exception on invalid formats.
  • Limit the amount of times the pygeoapi API is recreated.
  • Fix multiple configuration scenarios to use separate layers and clean the configuration cache.
  • Improve a variable name to avoid name shadowing.

@jdw-creare
Copy link
Copy Markdown
Contributor

This looks great, thanks Sam!

@jdw-creare jdw-creare merged commit bcc22a0 into develop Dec 12, 2025
5 checks passed
@scranford1 scranford1 deleted the feature/edr_server branch February 3, 2026 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants